Move object dunders from `FunctionModel` to `ObjectModel` #2847

emmanuel-ferdman · 2025-10-04T21:53:10Z

Type of Changes

	Type
✓	🐛 Bug fix

Description

This PR does:

This PR moves object dunder methods from FunctionModel to ObjectModel as Unknown placeholders, making them available to all object types.

Fixes #2742 #2741.

Signed-off-by: Emmanuel Ferdman <[email protected]>

codecov · 2025-10-04T21:59:30Z

Codecov Report

❌ Patch coverage is 93.02326% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.33%. Comparing base (2ce47a2) to head (c4f01a1).

Files with missing lines	Patch %	Lines
astroid/raw_building.py	25.00%	3 Missing ⚠️

❌ Your patch check has failed because the patch coverage (93.02%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2847      +/-   ##
==========================================
- Coverage   93.37%   93.33%   -0.04%     
==========================================
  Files          92       92              
  Lines       11150    11168      +18     
==========================================
+ Hits        10411    10424      +13     
- Misses        739      744       +5

Flag	Coverage Δ
linux	`93.20% <93.02%> (-0.04%)`	⬇️
pypy	`93.33% <93.02%> (-0.04%)`	⬇️
windows	`93.32% <93.02%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
astroid/bases.py	`89.74% <100.00%> (+0.05%)`	⬆️
astroid/interpreter/objectmodel.py	`96.33% <100.00%> (+0.06%)`	⬆️
astroid/nodes/scoped_nodes/scoped_nodes.py	`93.61% <100.00%> (+0.02%)`	⬆️
astroid/raw_building.py	`93.98% <25.00%> (-0.60%)`	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

DanielNoord

As can be seen from the docs you link, most of these should live on ObjectModel instead as they come from object.

We tried to do so before in #1519 but failed. Perhaps you want to have another look at that PR and see if you can fix the issue we faced there?

emmanuel-ferdman · 2025-10-05T16:38:03Z

@DanielNoord Thanks! I’ll take a look at this. From that thread, it seems you were close to a solution, except for one test case that didn’t pass. Do you remember which test it was?

DanielNoord · 2025-10-05T17:44:56Z

@emmanuel-ferdman I believe it was an issue in the numpy brain, which overwrites __eq__. That overwrite doesn't work well with the changes to ObjectModel.

Signed-off-by: Emmanuel Ferdman <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: Emmanuel Ferdman <[email protected]>

emmanuel-ferdman · 2025-10-06T00:02:24Z

@DanielNoord I've put together an initial solution for moving object dunders from FunctionModel to ObjectModel. I've tested it against both the astroid test suite and the pylint test suite (all passing). Since I'm still learning the codebase, I'd appreciate any guidance on whether this approach is correct.

The solution uses a placeholder pattern: ObjectModel provides Unknown placeholders for all 24 object dunders, and the lookup logic skips these placeholders to find actual implementations.
Key changes:

ObjectModel now has all object dunders as Unknown placeholders that act as fallbacks when no actual implementation exists
Lookup logic in BaseInstance.getattr() and ClassDef.getattr() skips Unknown placeholders and continues searching
Builtin dunders (from the builtins module) return Uninferable instead of raising InferenceError since we can't infer their result without executing C code
Special case for __hash__ = None to ensure unhashable types (list, dict, set) properly override object's __hash__
Enhanced type.__new__() validation to raise descriptive InferenceError instead of silently returning None

How it works:

When a class overrides a dunder: lookup finds the actual implementation and returns the bound method
When a class doesn't override: lookup finds the Unknown placeholder, skips it, continues searching, eventually returns the placeholder which yields Uninferable when called
For unhashable builtin types (list, dict, set): __hash__ is explicitly set to None in locals.

Thanks for any feedback 🙌

DanielNoord

This is amazing, very well written PR and a nice set of tests.

Well done on getting this to pass all tests. I have left some comments, but would really like to help you push this over the line :)

DanielNoord · 2025-10-06T08:55:47Z

astroid/nodes/scoped_nodes/scoped_nodes.py

+            # Builtin dunder methods have empty bodies, return Uninferable.
+            if (
+                self.root().qname() == "builtins"
+                and self.name.startswith("__")
+                and self.name.endswith("__")
+                and self.parent
+                and self.parent.__class__.__name__ == "ClassDef"
+            ):
+                yield util.Uninferable
+                return


I think I sort of understand this change, but don't understand why we need to special case it like this.

Can we replace the checks with a look up in special_attributes? Or doesn't that work?

We need this case because infer_call_result() has to decide what a call returns, and for built-in dunder methods there is no Python body to inspect (since they're implemented in C). If we tried to parse return nodes we would either fail or produce incorrect results.

special_attributes only tells us that an attribute exists (a placeholder), not whether there is a Python body to infer a return value from. Therefore it cannot replace the empty-body/builtins check used here.

But on second thought, I simplified the case by explicitly checking len(self.body) == 0.

Wouldn't it be okay if we always did this for len(self.body) == 0? Is there a function where len(self.body) == 0 that is inferable? Or is as ... or pass as body also len(self.body) == 0?

I don't think we can't simplify to just len(self.body) == 0 because both builtin dunders (__eq__, __hash__) and non-dunder builtins (getattr, len, print) have empty bodies. If we return Uninferable for all empty-body functions, it breaks tests - specifically getattr(int, 'missing_attr') returns Uninferable instead of raising InferenceError (test test_attribute_missing fails). Regarding pass/... - they create len(body) = 1, not 0. Only C builtins have truly empty bodies.

What I think I can simplify is to drop the self.root().qname() == "builtins" check since len(body) == 0 already guarantees it's a builtin. All tests pass (both in astroid and pylint) with this simplification. However, I'm not 100% certain there isn't some corner case we're missing with that removal. Let me know if you prefer me to drop this check 😄

DanielNoord · 2025-10-06T08:56:27Z

astroid/nodes/scoped_nodes/scoped_nodes.py

+            special_attr = self.special_attributes.lookup(name)
+            if not isinstance(
+                special_attr, (util.UninferableBase, node_classes.Unknown)
+            ):
+                result = [special_attr]
+                return result


What is the effect of this? What will we eventually return if the if is not True?

This block is a short-circuit used when there are no concrete locals/ancestor definitions: originally it always returned special_attributes.lookup(name) (even if that was Unknown or Uninferable). The new behavior only returns the special_attr when it is a concrete value (not node_classes.Unknown or util.UninferableBase), preventing a placeholder from being returned prematurely and masking an override in a metaclass/base class. If the if is not true we continue the normal lookup (metaclass lookup, collect/filter locals/ancestors) and ultimately return any real definitions found - otherwise an AttributeInferenceError is raised. Placeholders (Unknown/Uninferable) therefore mean “keep looking,” not “return this as the final result.”

Unkown feels like a nice placeholder for "keep looking", Uninferable generally means "stop inferring, you won't be able to". Can we make this only check for Unknown? Or does that not work?

Yes, good idea! ObjectModel only returns Unknown as placeholders, never Uninferable. I changed both checks (in ClassDef.getattr() and BaseInstance.getattr()) to only check for Unknown. If Uninferable somehow appeared from special_attributes, we should treat it as a final value and return it, not skip it. All tests pass with this change.

DanielNoord · 2025-10-06T08:56:38Z

astroid/bases.py

        except AttributeInferenceError as exc:
            if self.special_attributes and name in self.special_attributes:
-                return [self.special_attributes.lookup(name)]
+                special_attr = self.special_attributes.lookup(name)


Similar question as above

Same pattern as the ClassDef.getattr() check - when special_attr is Unknown or Uninferable, we skip the early return and continue to the if lookupclass: block which searches the class for the attribute. This ensures we find actual implementations in classes rather than stopping at the Unknown placeholder.

astroid/bases.py

DanielNoord · 2025-10-06T08:58:11Z

astroid/raw_building.py

+    # Special case: __hash__ = None overrides ObjectModel for unhashable types.
+    if name == "__hash__" and value is None:
+        _attach_local_node(node, nodes.const_factory(value), name)


Should this be the case for all attributes where value is None? Or only hash?

I believe only for __hash__. In Python __hash__ = None has a special semantic meaning - it explicitly marks a type as unhashable and must override the inherited object.__hash__. Other attributes set to None do not carry this override semantics and should not implicitly replace special_attributes entries, because that could hide real implementations or metadata. So I belive we should have this special-case restricted to __hash__ only.

Perhaps we can link that documentation here? It would prevent me from wondering why this special case is here the next time.

And thanks for the link, TIL about something I knew implicitly but now know where to find the documentation for that feature 😄 Appreciated!

Good idea - added the documentation link to both places 😄

DanielNoord · 2025-10-06T08:58:20Z

astroid/raw_building.py

                child = object_build_datadescriptor(node, member)
            elif isinstance(member, tuple(node_classes.CONST_CLS)):
-                if alias in node.special_attributes:
+                # Special case: __hash__ = None overrides ObjectModel for unhashable types.


Same question as above

Same reasoning as the previous hash question - hash = None is the only attribute where None has special semantic meaning in Python (marks unhashable types). Other None values don't need this override behavior.

DanielNoord · 2025-10-06T08:58:57Z

tests/test_inference.py

+    # Builtin dunder methods now return Uninferable instead of raising InferenceError
+    result = next(lenmeth.infer_call_result(None, None))
+    assert result is util.Uninferable


Ah, I think the changes above now make more sense to me.

pylint works correctly with this change? Thanks for testing that.

Yes - pylint works correctly (verified by running pylint test suite with astroid changes). pylint handles Uninferable in two ways: (1) explicit isinstance(value, util.UninferableBase) checks (e.g., pylint/checkers/typecheck.py:1365-1369, and (2) Uninferable is falsy like None, so if inferred: checks skip both. The behavior is identical - when builtin dunders can't be inferred, pylint skips type checking whether it gets None from an exception or Uninferable from the return value.

DanielNoord · 2025-10-06T08:59:31Z

tests/test_object_model.py

+    )
+    inferred = next(eq_result.infer())
+    assert isinstance(inferred, nodes.Const)
+    assert inferred.value == "custom equality"


Signed-off-by: Emmanuel Ferdman <[email protected]>

Add missing operators in FunctionModel

aeedd50

Signed-off-by: Emmanuel Ferdman <[email protected]>

Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label Oct 5, 2025

Pierre-Sassoulas added this to the 4.1.0 milestone Oct 5, 2025

DanielNoord reviewed Oct 5, 2025

View reviewed changes

emmanuel-ferdman and others added 3 commits October 6, 2025 02:48

Move object dunders from FunctionModel to ObjectModel

50a56f8

Signed-off-by: Emmanuel Ferdman <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

b691e28

for more information, see https://pre-commit.ci

Move object dunders from FunctionModel to ObjectModel

1bc57c5

Signed-off-by: Emmanuel Ferdman <[email protected]>

DanielNoord reviewed Oct 6, 2025

View reviewed changes

Move object dunders from FunctionModel to ObjectModel

2762ac9

Signed-off-by: Emmanuel Ferdman <[email protected]>

emmanuel-ferdman changed the title ~~Add missing operators in FunctionModel~~ Move object dunders from FunctionModel to ObjectModel Oct 7, 2025

emmanuel-ferdman mentioned this pull request Oct 7, 2025

Make type.__new__() raise clear errors instead of returning None #2851

Merged

emmanuel-ferdman added 2 commits October 8, 2025 00:49

Merge branch 'pylint-dev:main' into main

6c4d29f

Move object dunders from FunctionModel to ObjectModel

c4f01a1

Signed-off-by: Emmanuel Ferdman <[email protected]>

Uh oh!

Move object dunders from FunctionModel to ObjectModel #2847

Are you sure you want to change the base?

Move object dunders from FunctionModel to ObjectModel #2847

Conversation

emmanuel-ferdman commented Oct 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of Changes

Description

Uh oh!

codecov bot commented Oct 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DanielNoord left a comment

Choose a reason for hiding this comment

Uh oh!

emmanuel-ferdman commented Oct 5, 2025

Uh oh!

DanielNoord commented Oct 5, 2025

Uh oh!

emmanuel-ferdman commented Oct 6, 2025

Uh oh!

DanielNoord left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emmanuel-ferdman Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Move object dunders from `FunctionModel` to `ObjectModel` #2847

Move object dunders from `FunctionModel` to `ObjectModel` #2847

emmanuel-ferdman commented Oct 4, 2025 •

edited

Loading

codecov bot commented Oct 4, 2025 •

edited

Loading

emmanuel-ferdman Oct 7, 2025 •

edited

Loading